Stop claiming §5.6 covers the failure-isolation marker span - #287
Merged
chris-colinsky merged 1 commit intoAug 30, 2026
Merged
Conversation
Spec checked the obligation and could not find the span: openarmature.failure_isolated appears nowhere in the observability spec, and 5.6's scope sentence enumerates the spans it reaches without including it. The failure-isolation EVENT is mandated by pipeline-utilities; the span we emit from it is ours and unmapped. The claim was asserted in four places: both observers' comments, the CHANGELOG entry and a test comment. Each now says the caller set is there for cross-observer consistency, and that spec is drafting a mapping which may land differently. No behaviour change. The metadata still reaches both markers, and the mutation that removes it still turns the Langfuse test red. EOF
There was a problem hiding this comment.
Pull request overview
This PR corrects prior commentary that incorrectly cited observability spec §5.6 as requiring the cross-cutting caller metadata to be applied to the openarmature.failure_isolated marker span, clarifying that doing so is a cross-observer consistency choice rather than spec conformance.
Changes:
- Update OTel and Langfuse observer inline comments to remove the §5.6 conformance claim and explain the rationale as consistency across observers.
- Update the unit test comment to explicitly pin the current consistency decision while acknowledging a future spec-defined mapping may change the assertion.
- Amend the v0.17.0 changelog entry to remove the incorrect §5.6 justification for the marker span metadata.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/unit/test_failure_isolation_middleware.py | Updates test commentary to remove the incorrect §5.6 rationale and document the pinned consistency choice. |
| src/openarmature/observability/otel/observer.py | Rewords the failure-isolation marker span comment to reflect consistency (not §5.6 conformance). |
| src/openarmature/observability/langfuse/observer.py | Mirrors the same clarification on the Langfuse observer side. |
| CHANGELOG.md | Corrects the v0.17.0 entry to remove the false §5.6 claim for the marker span. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
chris-colinsky
deleted the
fix/failure-isolated-span-is-not-spec-defined
branch
August 30, 2026 18:59
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Comments and changelog only. No behaviour change.
The claim was false
#285 put the cross-cutting
openarmature.user.*set on theopenarmature.failure_isolatedmarker span and justified it with observability §5.6, "the cross-cutting set goes on EVERY span emitted during the invocation".Spec went to check that obligation and could not find the span. Verified independently before conceding:
openarmature.failure_isolatedappears zero times inspec/observability/spec.mdspec/eitherThe failure-isolation event is mandated by pipeline-utilities. The span we emit from it is ours, and has no mapping in the spec at all.
What changed
The claim was asserted in four places, all corrected:
otel/observer.py— the comment above_apply_caller_metadataon the markerlangfuse/observer.py— the same comment on its sideCHANGELOG.md— the[0.17.0]Fixed entry for Recover the subgraph identity and caller metadata a short-circuiting wrapper loses #285test_failure_isolation_middleware.py— the new test's commentEach now says the caller set is there for cross-observer consistency rather than conformance, and that spec is drafting a mapping which may land differently. The test comment says explicitly that if the mapping lands differently, the assertion changes with it.
Why it matters more than a wording fix
These comments carry reasoning. A reader checking why the metadata is applied would have found a §5.6 citation, gone to §5.6, and either concluded the code was right for a reason it isn't, or concluded the spec was missing something it never claimed. The CHANGELOG entry told the same story to anyone reading the release notes.
The underlying gap is real and now spec's: a framework-emitted event the spec mandates, with no defined mapping for what an observer should render from it, means every implementation invents one. We invented a span name, attribute set and parent. Tracked in coord
release-v0.17.0/47; we agreed in49that it should be spec-defined even if it lands differently from ours.Verification
No behaviour change: the metadata still reaches both markers, and the mutation that removes the Langfuse call still turns
test_langfuse_failure_isolated_marker_carries_caller_metadatared.2196 passing; ruff, format and pyright clean. A sweep confirms no surviving claim that §5.6 covers this span.